Fix: tenure downloader reward set error #6234 #6276
Merged
+350
−46
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
obycode
previously approved these changes
Jul 10, 2025
obycode
previously approved these changes
Jul 11, 2025
Jiloc
previously approved these changes
Jul 11, 2025
Co-authored-by: Francesco <2530388+Jiloc@users.noreply.github.com>
obycode
approved these changes
Jul 11, 2025
github-project-automation
bot
moved this from Status: In Review
to Status: 💻 In Progress
in Stacks Core Eng
Merged
via the queue into
stacks-network:develop
with commit Jul 14, 2025
fbb3ae6
240 of 241 checks passed
github-project-automation
bot
moved this from Status: 💻 In Progress
to Status: ✅ Done
in Stacks Core Eng
Codecov ReportAttention: Patch coverage is
❌ Your project status has failed because the head coverage (77.96%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #6276 +/- ##
===========================================
- Coverage 82.10% 77.96% -4.14%
===========================================
Files 546 546
Lines 347235 347462 +227
Branches 323 323
===========================================
- Hits 285088 270897 -14191
- Misses 62139 76557 +14418
Partials 8 8
... and 218 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refactors the reward cycle fetching logic in the tenure downloader and fixes the #6234 bug.
First -- I moved the
downloader_block_height_to_reward_cyclemethod into a newly named (and commented) method thats a static method of the NakamotoDownloadStateMachine struct. The logic of that calculation is: given a block commit at burn height X, what is the latest possible reward cycle for a tenure the commit confirms?I think that the only places we want to use this calculation are in
WantedTenuresrequest generation -- basically, given a sortition tip, what tenures could we download and confirm?The bug in #6234 was caused by the fact that this calculation was also used for the reward set caching logic of the (unconfirmed) tenure downloader. This meant that for the first two tenures of a new reward cycle, the wrong reward sets would be used for download validation. The fix for this was to simply use the normal reward set calculation in those instances.
This PR adds an integration test for this behavior: basically, perform signer-set rollover with 2 active nodes. Importantly, in order for the test to actually cover the behavior, the nodes need to refuse block pushes (otherwise, they never need to use the TenureDownloader at all).